Skip to content

[Multithreaded] Migrate RunReadyToRunCompiler#55339

Open
AlesProkop wants to merge 3 commits into
dotnet:mainfrom
AlesProkop:migrate-run-ready-to-run-compiler-task
Open

[Multithreaded] Migrate RunReadyToRunCompiler#55339
AlesProkop wants to merge 3 commits into
dotnet:mainfrom
AlesProkop:migrate-run-ready-to-run-compiler-task

Conversation

@AlesProkop

Copy link
Copy Markdown
Member

Related to dotnet/msbuild#13073

Context

RunReadyToRunCompiler used process-global state when resolving tool, input, and output paths. The task needs per-project path and process environment handling before it can execute safely as a multithreaded MSBuild task.

Changes Made

  • Added MSBuildMultiThreadableTask and IMultiThreadableTask support with a TaskEnvironment.Fallback default.
  • Absolutized filesystem and executable paths through TaskEnvironment while preserving original values in diagnostics and compiler arguments.
  • Added a .NET Framework ToolTask.GetProcessStartInfo compatibility override to apply the task-local working directory and environment.
  • Added focused multithreading tests for independent project-directory resolution and empty tool-path diagnostics.

Testing

  • Built Microsoft.NET.Build.Tasks.csproj for net11.0 successfully.
  • Built Microsoft.NET.Build.Tasks.csproj for net472 successfully.
  • Added and ran focused tests in GivenARunReadyToRunCompilerMultiThreading:
    • RelativePathsResolveIndependentlyForEachTask verifies relative paths resolve against each task instance's project directory.
    • EmptyToolPathUsesExistingValidationDiagnostic verifies empty paths retain the existing validation failure behavior.
    • Result: 2 passed, 0 failed.

@azure-pipelines

Copy link
Copy Markdown
Azure Pipelines:
Successfully started running 1 pipeline(s).
2 pipeline(s) were filtered out due to trigger conditions.
There may be pipelines that require an authorized user to comment /azp run to run.

@AlesProkop
AlesProkop marked this pull request as ready for review July 24, 2026 14:37
@AlesProkop
AlesProkop requested a review from a team as a code owner July 24, 2026 14:37
Copilot AI review requested due to automatic review settings July 24, 2026 14:37
@azure-pipelines

Copy link
Copy Markdown
Azure Pipelines:
Successfully started running 1 pipeline(s).
2 pipeline(s) were filtered out due to trigger conditions.
There may be pipelines that require an authorized user to comment /azp run to run.

Copilot AI left a comment

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Pull request overview

Migrates RunReadyToRunCompiler to be safe for multithreaded MSBuild execution by moving path/environment resolution onto MSBuild’s per-task TaskEnvironment model and adding focused concurrency tests.

Changes:

  • Marked RunReadyToRunCompiler as [MSBuildMultiThreadableTask] and implemented IMultiThreadableTask, switching path resolution to TaskEnvironment.GetAbsolutePath(...).
  • Added a .NET Framework ToolTask.GetProcessStartInfo override to apply task-local working directory and environment variables.
  • Added multithreading-focused unit tests validating per-task relative-path resolution and empty-tool-path diagnostics.

Reviewed changes

Copilot reviewed 2 out of 2 changed files in this pull request and generated 1 comment.

File Description
test/Microsoft.NET.Build.Tasks.Tests/GivenARunReadyToRunCompilerMultiThreading.cs Adds multithreading-focused unit tests for task-local path resolution and empty-tool-path diagnostic behavior.
src/Tasks/Microsoft.NET.Build.Tasks/RunReadyToRunCompiler.cs Converts tool/input/output path checks to use TaskEnvironment and adds a .NET Framework GetProcessStartInfo override for task-local process environment.

Comment on lines +202 to 206
string jitPath = CrossgenTool.GetMetadata(MetadataKeys.JitPath);
if (string.IsNullOrEmpty(jitPath) ||
!File.Exists(TaskEnvironment.GetAbsolutePath(jitPath)))
{
Log.LogError(Strings.JitLibraryNotFound, MetadataKeys.JitPath);
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

2 participants